Introduce Reply Paths for BOLT12 Invoice in Offers Flow. #3163

shaavan · 2024-07-09T14:39:39Z

This PR builds on #3087 and addresses this comment.

Changes:

Updates the Offers message flow to include reply_path with the sent BOLT12Invoice.
Ensures that in case of an error, the counterparty can send back any InvoiceError along the reply_path.
Updates the Offers test to check for the invoice's reply_path wherever applicable.

codecov-commenter · 2024-07-09T14:47:24Z

Codecov Report

Attention: Patch coverage is 77.04918% with 14 lines in your changes missing coverage. Please review.

Project coverage is 90.39%. Comparing base (5e62df7) to head (7b49993).
Report is 62 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/channelmanager.rs	66.66%	9 Missing ⚠️
lightning/src/offers/signer.rs	68.75%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3163      +/-   ##
==========================================
+ Coverage   89.82%   90.39%   +0.57%     
==========================================
  Files         126      126              
  Lines      103024   108017    +4993     
  Branches   103024   108017    +4993     
==========================================
+ Hits        92543    97644    +5101     
- Misses       7769     7815      +46     
+ Partials     2712     2558     -154

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

lightning/src/ln/channelmanager.rs

lightning/src/ln/offers_tests.rs

TheBlueMatt · 2024-07-16T20:14:38Z

Needs rebase now.

shaavan · 2024-07-17T13:27:51Z

Updated from pr3163.01 to pr3163.02 (diff):
Addressed @jkczyz and @TheBlueMatt comments

Changes:

Rebase on main.
Introduce a fixup commit to introduce context in respond_with_reply_path.

TheBlueMatt · 2024-07-17T14:54:50Z

Please include details about what you're doing and why in the commit messages.

jkczyz · 2024-07-17T14:58:51Z

Please include details about what you're doing and why in the commit messages.

Sorry about being lax when reviewing this. @shaavan I recommend this reference, which we mention in the project contributor guidelines: https://cbea.ms/git-commit/

TheBlueMatt · 2024-07-17T15:01:16Z

I'm usually pretty lax as long as its clear from the code what's going on, but in this case I look at the commit and we're just adding a blinded path, which doesn't tell me anything about why. I have some context from a separate commit in a different PR, but I don't want to rely on that :)

shaavan · 2024-07-18T10:27:22Z

Updated from pr3163.02 to pr3163.03 (diff):
Addressed @TheBlueMatt and @jkczyz comments

Changes:

Squashed the commits
Update the Commit description to be clearly detailed.

Thanks, @TheBlueMatt for the suggestion, and @jkczyz for the resource!

TheBlueMatt · 2024-08-06T15:47:22Z

lightning/src/ln/channelmanager.rs

 				match response {
-					Ok(invoice) => responder.respond(OffersMessage::Invoice(invoice)),
+					Ok(invoice) => {
+						let context = MessageContext::Offers(OffersContext::Unknown {});


What are we actually gonna do with the InvoiceError now? This is for an inbound payment, presumably we'll just log and move on? Do we want to authenticate the BP at all? @jkczyz

Yeah, I'd figure we'd just log. What is "BP" refer to here?

Oh, blinded path. 😛 We could though maybe a lower log level is fine for now. Not sure how much we can do about sending another invoice without another request.

shaavan · 2024-08-20T12:29:40Z

Updated from pr3163.03 to pr3163.04 (diff):

Changes:

Rebase on main, to resolve merge conflicts.

shaavan · 2024-08-21T09:44:43Z

Updated from pr3163.04 to pr3163.05 (diff):

Changes:

Moved the relevant change to the relevant commit to have a clean commit history.

shaavan · 2024-08-28T08:22:29Z

Updated from pr3163.05 to pr3163.06 (diff):

Changes:

Rebase on main.

shaavan · 2024-08-28T08:30:44Z

Updated from pr3163.06 to pr3163.07 (diff):
Addressed @jkczyz comment

Update the check in a test to maintain channel setup consistency with the other tests.

lightning/src/ln/offers_tests.rs

shaavan · 2024-08-29T11:39:27Z

Updated from pr3163.07 to pr3163.08 (diff):
Addressed @jkczyz comment

Remove an extra redundant node introduced earlier in the test.

lightning/src/ln/channelmanager.rs

When a InvoiceError is received for a sent BOLT12Invoice, the corresponding PaymentHash is to be logged. Introduce hmac construction and verification function for PaymentHash for this purpose.

shaavan · 2024-09-06T12:34:24Z

Updated from pr3163.08 to pr3163.09 (diff):
Addressed @TheBlueMatt comment

Changes:

Introduce HMAC calculation and verification for OffersContext::InboundPayment.

lightning/src/blinded_path/message.rs

lightning/src/ln/channelmanager.rs

shaavan · 2024-09-09T12:26:15Z

Updated from pr3163.09 to pr3163.10 (diff):
Addressed @jkczyz comments

Changes:

Update docs.
Always use new nonce for the reply path sent with Bolt12Invoice.

- The trait defines the public method one may define for creating and verifying the HMAC. - Using a pub trait to define these method allows the flexibility for other `OffersMessageHandler` construct to construct the HMAC and authenticate the message.

shaavan · 2024-09-10T10:30:51Z

Updated from pr3163.10 to pr3163.11 (diff):
Addressed @jkczyz comment

Changes:

Introduce Verification trait that define the public function required for creating and verifying HMAC.
Define the trait impl for PaymentId, and PaymentHash.

TheBlueMatt

LGTM, feel free to squash.

Introduce HMAC and nonce calculation when sending Invoice with reply path, so that if we receive InvoiceError back for the corresponding Invoice we can verify the payment hash before logging it.

1. Introduced reply_path in BOLT12Invoices to address a gap in error handling. Previously, if a BOLT12Invoice sent in the offers flow generated an Invoice Error, the payer had no way to send this error back to the payee. 2. By adding a reply_path to the Invoice Message, the payer can now communicate any errors back to the payee, ensuring better error handling and communication within the offers flow.

1. Updated the Offers Test to check the reply paths in BOLT12 Invoices. 2. Changed the `extract_invoice` return type from `Option<BlindedMessagePath>` to `BlindedMessagePath` since all BOLT12Invoices now have a corresponding reply path by default.

shaavan · 2024-09-11T13:26:24Z

Updated from pr3163.11 to pr3163.12 (diff):
Addressed @TheBlueMatt comment

Changes:

Squash fixups.

shaavan mentioned this pull request Jul 9, 2024

Allow blinded path diversification by expanding create_blinded_paths #3087

Merged

jkczyz self-requested a review July 9, 2024 14:54

jkczyz reviewed Jul 9, 2024

View reviewed changes

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved

lightning/src/ln/offers_tests.rs Outdated Show resolved Hide resolved

shaavan mentioned this pull request Jul 16, 2024

Authenticate use of offer blinded paths #3139

Merged

shaavan force-pushed the invoice_reply_path branch from 1a6175e to eb0d3cc Compare July 17, 2024 13:27

shaavan force-pushed the invoice_reply_path branch from eb0d3cc to c2dd2fd Compare July 18, 2024 10:27

TheBlueMatt reviewed Aug 6, 2024

View reviewed changes

shaavan force-pushed the invoice_reply_path branch from c2dd2fd to 890607e Compare August 20, 2024 12:29

shaavan force-pushed the invoice_reply_path branch from 890607e to bd171ee Compare August 21, 2024 09:44

jkczyz mentioned this pull request Aug 22, 2024

Remove message type bound on ResponseInstruction #3263

Merged

shaavan force-pushed the invoice_reply_path branch from bd171ee to 213bafd Compare August 28, 2024 08:22

shaavan force-pushed the invoice_reply_path branch from 213bafd to f84a00e Compare August 28, 2024 08:30

jkczyz reviewed Aug 28, 2024

View reviewed changes

lightning/src/ln/offers_tests.rs Outdated Show resolved Hide resolved

shaavan force-pushed the invoice_reply_path branch from f84a00e to e74642d Compare August 29, 2024 11:39

jkczyz previously approved these changes Aug 30, 2024

View reviewed changes

TheBlueMatt reviewed Sep 5, 2024

View reviewed changes

lightning/src/ln/channelmanager.rs Outdated Show resolved Hide resolved

valentinewallace mentioned this pull request Sep 5, 2024

Support paying static invoices #3140

Merged

HMAC Construction and Verification for PaymentHash

e2ee325

When a InvoiceError is received for a sent BOLT12Invoice, the corresponding PaymentHash is to be logged. Introduce hmac construction and verification function for PaymentHash for this purpose.

shaavan dismissed jkczyz’s stale review via 2d794b5 September 6, 2024 12:34

shaavan force-pushed the invoice_reply_path branch from e74642d to 2d794b5 Compare September 6, 2024 12:34

jkczyz reviewed Sep 6, 2024

View reviewed changes

shaavan force-pushed the invoice_reply_path branch from 2d794b5 to c794bdd Compare September 9, 2024 12:26

shaavan force-pushed the invoice_reply_path branch from c794bdd to e8b2409 Compare September 10, 2024 10:24

Introduce Verification trait.

6500277

- The trait defines the public method one may define for creating and verifying the HMAC. - Using a pub trait to define these method allows the flexibility for other `OffersMessageHandler` construct to construct the HMAC and authenticate the message.

shaavan force-pushed the invoice_reply_path branch from e8b2409 to e9a3e2c Compare September 10, 2024 10:30

TheBlueMatt reviewed Sep 10, 2024

View reviewed changes

shaavan added 3 commits September 11, 2024 18:54

Add HMAC, and nonce to OffersContext::InboundPayment

8b479ac

Introduce HMAC and nonce calculation when sending Invoice with reply path, so that if we receive InvoiceError back for the corresponding Invoice we can verify the payment hash before logging it.

shaavan force-pushed the invoice_reply_path branch from e9a3e2c to 7b49993 Compare September 11, 2024 13:26

jkczyz approved these changes Sep 11, 2024

View reviewed changes

TheBlueMatt approved these changes Sep 11, 2024

View reviewed changes

TheBlueMatt merged commit 4178dd7 into lightningdevkit:main Sep 11, 2024

shaavan deleted the invoice_reply_path branch September 12, 2024 12:56

Introduce Reply Paths for BOLT12 Invoice in Offers Flow. #3163

Introduce Reply Paths for BOLT12 Invoice in Offers Flow. #3163

Uh oh!

Conversation

shaavan commented Jul 9, 2024

Changes:

Uh oh!

codecov-commenter commented Jul 9, 2024 • edited by codecov bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

TheBlueMatt commented Jul 16, 2024

Uh oh!

shaavan commented Jul 17, 2024

Uh oh!

TheBlueMatt commented Jul 17, 2024

Uh oh!

jkczyz commented Jul 17, 2024

Uh oh!

TheBlueMatt commented Jul 17, 2024

Uh oh!

shaavan commented Jul 18, 2024

Uh oh!

TheBlueMatt Aug 6, 2024

Choose a reason for hiding this comment

Uh oh!

jkczyz Aug 20, 2024

Choose a reason for hiding this comment

Uh oh!

jkczyz Aug 20, 2024

Choose a reason for hiding this comment

Uh oh!

shaavan commented Aug 20, 2024

Uh oh!

shaavan commented Aug 21, 2024

Uh oh!

shaavan commented Aug 28, 2024

Uh oh!

shaavan commented Aug 28, 2024

Uh oh!

Uh oh!

shaavan commented Aug 29, 2024

Uh oh!

Uh oh!

shaavan commented Sep 6, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shaavan commented Sep 9, 2024

Uh oh!

shaavan commented Sep 10, 2024

Uh oh!

TheBlueMatt left a comment

Choose a reason for hiding this comment

Uh oh!

shaavan commented Sep 11, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-commenter commented Jul 9, 2024 •

edited by codecov bot

Loading